Skip to content

Feature/rate limit login - #64

Merged
cchwala merged 3 commits into
mainfrom
feature/rate-limit-login
Aug 3, 2026
Merged

Feature/rate limit login#64
cchwala merged 3 commits into
mainfrom
feature/rate-limit-login

Conversation

@cchwala

@cchwala cchwala commented Jul 30, 2026

Copy link
Copy Markdown
Member

Overview

Adds security hardening through rate limiting on sensitive endpoints and fixes test infrastructure issues.

Changes

Security Features

  • Rate limiting: Added Flask-Limiter to protect against brute-force and DoS attacks
  • Login endpoint: Limited to 5 requests/minute to prevent credential stuffing
  • File upload endpoint: Limited to 10 requests/minute to prevent upload flooding
  • File listing endpoint: Limited to 30 requests/minute to prevent enumeration attacks
  • 429 error handler: Returns JSON response when rate limits exceeded
  • Configurable storage: Supports memory (default) or Redis via RATE_LIMIT_STORAGE_URI env var

Infrastructure

  • Docker Compose: Added rate limit configuration support
  • Dependencies: Added flask-limiter to webserver requirements

Reverse Proxy Support

  • ProxyFix middleware: Unwraps X-Forwarded-For so rate limiting keys on the real client IP, not the nginx IP (which would put all users in the same bucket)
  • PROXY_COUNT env var: Defaults to 0 (direct); set to 1 in the deploy override when running behind nginx

Test Fixes

  • Grafana proxy tests: Fixed 3 failing CI tests by updating authentication mock pattern
  • Test consistency: Aligned logged_in_client fixture with pattern used in 90+ passing tests

Testing

All 95 webserver tests pass.

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.65%. Comparing base (f805562) to head (904ffca).

Files with missing lines Patch % Lines
webserver/main.py 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
+ Coverage   85.39%   86.65%   +1.25%     
==========================================
  Files          41       37       -4     
  Lines        3678     3312     -366     
==========================================
- Hits         3141     2870     -271     
+ Misses        537      442      -95     
Flag Coverage Δ
mno_simulator 86.12% <ø> (ø)
parser 90.14% <ø> (ø)
scripts ?
webserver 75.40% <92.85%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…acks

- Add Flask-Limiter dependency for rate limiting
- Limit login endpoint to 5 requests/minute (prevents credential stuffing)
- Limit file upload to 10 requests/minute (prevents upload flooding)
- Limit file listing to 30 requests/minute (prevents enumeration)
- Add 429 error handler with JSON response
- Support memory storage by default, configurable via RATE_LIMIT_STORAGE_URI env var

Security impact: Mitigates brute-force attacks on login and resource exhaustion via API endpoints.
@cchwala
cchwala force-pushed the feature/rate-limit-login branch from 52b47f0 to b3f2eaf Compare July 30, 2026 09:04
cchwala added 2 commits August 3, 2026 22:42
The logged_in_client fixture was using actual session-based login which
doesn't persist properly in test client when mocking is involved. This
caused mock_requests.request.call_args to be None, resulting in
'TypeError: cannot unpack non-iterable NoneType object' errors.

Updated to match pattern used in other passing tests (test_api_routes.py,
test_time_slider_api.py):
- Set LOGIN_DISABLED=True to bypass auth checks
- Mock current_user directly instead of relying on session
- Remove client.post('/login', ...) call that wasn't persisting

Fixes 3 failing CI tests:
- test_grafana_proxy_injects_webauth_user_header
- test_grafana_proxy_strips_client_webauth_user_header
- test_grafana_proxy_forwards_path
…ting

Without ProxyFix, get_remote_address returns the nginx IP, putting all
clients in the same rate limit bucket. Set PROXY_COUNT=1 in the deploy
docker-compose override when running behind nginx.
@cchwala
cchwala merged commit 6a41596 into main Aug 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant